Schedules redesign 5/5: view page, list, and action modals#3552
Schedules redesign 5/5: view page, list, and action modals#3552tegan-temporal wants to merge 6 commits into
Conversation
Replaces the schedule view page with card-based components (spec, advanced settings, workflow runs, input, search attributes), adds the trigger/backfill/pause modals behind a conditionally-mounted host so modal state resets on every open, and moves the list components under schedules-list/. The backfill modal interprets times in the schedule's timezone and validates that the end time follows the start time. Removes the superseded view components, legacy schedule types and i18n keys, and schedule-spec-label. Adds action-modal integration coverage and extends the e2e spec to a full create/verify/delete loop. Co-Authored-By: Claude Fable 5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The payload input now validates JSON for json/plain encoding, so submit a JSON string, and the schedule heading contains the status badge, so match on containment. Co-Authored-By: Claude Fable 5 <[email protected]>
7e0c7aa to
889107d
Compare
laurakwhit
left a comment
There was a problem hiding this comment.
Looking good! Noticed a few small things, but no major blockers.
| return runs | ||
| .filter(Boolean) | ||
| .sort( | ||
| (a, b) => getMilliseconds(a.actualTime) - getMilliseconds(b.actualTime), |
There was a problem hiding this comment.
Do we want the newest (not the oldest) to show here?
| (a, b) => getMilliseconds(a.actualTime) - getMilliseconds(b.actualTime), | |
| (a, b) => getMilliseconds(b.actualTime) - getMilliseconds(a.actualTime), |
| </dd> | ||
| </div> | ||
|
|
||
| {#if state.limitedActions} |
There was a problem hiding this comment.
| {#if state.limitedActions} | |
| {#if state?.limitedActions} |
| data-testid="schedule-name" | ||
| > | ||
| <WorkflowStatus | ||
| status={schedule?.schedule.state.paused ? 'Paused' : 'Running'} |
There was a problem hiding this comment.
| status={schedule?.schedule.state.paused ? 'Paused' : 'Running'} | |
| status={schedule?.schedule?.state?.paused ? 'Paused' : 'Running'} |
| </dd> | ||
| </div> | ||
|
|
||
| {#if schedule?.info.updateTime} |
There was a problem hiding this comment.
| {#if schedule?.info.updateTime} | |
| {#if schedule?.info?.updateTime} |
|
|
||
| let { input, scheduleId }: { input: Payloads; scheduleId: string } = $props(); | ||
| interface Props { | ||
| input: Payloads; |
There was a problem hiding this comment.
NIT: Can the input be null here?
| {translate('schedules.schedule-specs')} | ||
| </h2> | ||
| <Button | ||
| variant="ghost" |
There was a problem hiding this comment.
NIT: Wondering if this should be a "secondary" button or maybe just have an up/down chevron icon? Or Maybe a toggle would be more intuitive here?
| </h1> | ||
| </div> | ||
| </header> | ||
| <Loading /> |
There was a problem hiding this comment.
NIT: Instead of a loading spinner, wondering if we should follow the pattern of a skeleton loader we have elsewhere when loading a full page.
| <dd class="inline-flex items-center gap-1"> | ||
| {schedule?.schedule?.action?.startWorkflow?.workflowType?.name} | ||
| <div class="flex items-center gap-2"> | ||
| <Link |
There was a problem hiding this comment.
Could we use the DetailList components here? E.g. DetailListLinkValue.
| </header> | ||
|
|
||
| <PillContainer class="mr-auto flex flex-row p-1"> | ||
| <Pill |
There was a problem hiding this comment.
Curious why pills instead of our ToggleButtons or Tabs?
| <span class="font-mono" data-testid="workflow-count" | ||
| >{$workflowCount.count.toLocaleString()} | ||
| </span> | ||
| <Button |
There was a problem hiding this comment.
Can we use the existing CountRefreshButton here?
Description & motivation 💭
Final PR of the stack. After this merges,
feature/schedulesis byte-identical totegan/schedules-update(verified with an emptygit diff).schedule-view/card-based page: spec card with full-spec toggle, advanced settings, recent/upcoming workflow runs, workflow input, custom search attributesschedule-action-modals/: trigger, backfill, and pause modals rendered through a conditionally-mounted host keyed on the confirmation-modal store, so every open starts with fresh state (no stale selections, reasons, or page-load timestamps). Trigger pre-selects Allow All (matching server behavior for manual triggers) and both overlap pickers mark the schedule's current policyschedules-list/schedule-spec-label, legacy schedule types (ScheduleParametersfamily,Unspecifiedoverlap policy), and legacy i18n keysTesting 🧪
How was this tested 👻
pnpm check0 errors,pnpm lint0 errors, 2,080 unit tests passing, full integration suite 233 passed / 0 failed. Newschedule-actions.spec.tscovers trigger patch bodies, backfill request + post-success modal usability, modal state reset on reopen, timezone caption, end-before-start validation, and delete. The e2e spec now does a full create → verify summary → delete loop but needs a run against a real Temporal server (not runnable locally).Steps for others to test: 🚶🏽♂️🚶🏽♀️
On a schedule view page: trigger, backfill (try an end time before the start time, and reopen modals to confirm state resets), pause/unpause with a reason, and delete.
Checklists
Merge Checklist
feature/schedulestomainfeature/schedules(notmain)🤖 Generated with Claude Code